Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wsg eth test finished #21

Closed
wants to merge 7 commits into from

Conversation

devlancer412
Copy link
Contributor

No description provided.

Copy link
Owner

@chimera-defi chimera-defi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we plz add checks for price per share, and see that is behaves with new yield coming in + new deposits coming in/total supply increasing


const {r, s, v} = ethers.utils.splitSignature(signature);

// await expect(wsgEth.connect(alice).depositWithSignature(parseEther("1"), alice.address, deadline, false, v, r, s))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this not work as expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. You can check this issue.
#19

@devlancer412
Copy link
Contributor Author

can we plz add checks for price per share, and see that is behaves with new yield coming in + new deposits coming in/total supply increasing

I think this rate increasing by sgEth and wsgEth amount. but just deposit and withdraw keeps this rate to 1. How can I change this rate when test?

@chimera-defi
Copy link
Owner

@devlancer412 yep!
so the way it works is:

  1. user mints sgETH w/ eth. At the rate 1:1
  2. user mints/wraps to wsgETH w/ sgETH at the curRate
  3. Rewards generated via rewardsRecvr are redirected, sgETH is minted, but is then sent to the wsgeth contract.
  4. When rewards are sent, the redemption rate for wsgETH goes up
  5. user can now withdraw -> wsgETH to sgETH , at a better rate, receiving more than 1 sgETH
  6. User can now redeem his >1 sgETH for equivalent eth at 1:1

So for test, you could just admin mint some sgETH and send it to the wsgeth contract to test.

See it in action in the rewards receiver here for e2e: https://github.com/chimera-defi/SharedDeposit/blob/main/contracts/v2/core/RewardsReceiver.sol#L32

@devlancer412
Copy link
Contributor Author

devlancer412 commented Jun 6, 2024

So I need to deposit manually eth to RewardsReceiver contract to test sgEth/wsgEth rate change?

@chimera-defi
Copy link
Owner

@devlancer412 yep, exactly

@devlancer412 devlancer412 requested a review from chimera-defi June 6, 2024 05:00
@devlancer412
Copy link
Contributor Author

#23

I couldn't test that because this error

@devlancer412
Copy link
Contributor Author

I finished test by avoiding top error.

@devlancer412
Copy link
Contributor Author

@chimera-defi

await sgEth.approve(wsgEth.address, parseEther("2"));
await wsgEth.deposit(parseEther("2"), alice.address);

console.log(formatEther(await wsgEth.pricePerShare()));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove the left over console log

value: parseEther("1"),
});
// sends 60% of sgEth to WSGEth contract - so current rate is 1.5/2.1
console.log(await rewardsReceiver.state());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing log

// redeem will get 1.1 sgEth
await expect(wsgEth.connect(alice).redeem(parseEther("0.5"), alice.address, alice.address))
.to.be.emit(wsgEth, "Withdraw")
.withArgs(alice.address, alice.address, alice.address, parseEther("0.7"), parseEther("0.5"));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we checking the invariant? i.e. alice has the expected balance after withdraw?
im not sure how the 10% bonus is represented here, 0.5 -> 0.55 would be expected based on the comment

also are you sure you need to manually call syncRewards?
iirc it should be triggered on redeem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with my analysis, reward of n' epoch distributes to next round by linearly. To test easily, I should call this once.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah cool yea updated comment looks good

await expect(wsgEth.syncRewards()).to.be.revertedWith("SyncError()");
// increase time by reward cycle
await time.increase(24 * 60 * 60);
await wsgEth.syncRewards();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this call necessary? it should be triggered when redeem/withdraw is called automatically. and its a error condition if it needs to be manually triggered instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually syncRewards calls when user call another functions such as deposit and withdraw/redeem.
so actually not sure this function is called on time. But I should call this function to test increment of reward

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should work without the direct call. when the user calls another function
or try to call the following again after sync rewards:

    // deposit to rewardReceiver to simulate reward
    await deployer.sendTransaction({
      to: rewardsReceiver.address,
      value: parseEther("1"),
    });
    // sends 60% of sgEth to WSGEth contract - so current rate is 1.5/2.1
    await rewardsReceiver.work();

@devlancer412
Copy link
Contributor Author

@chimera-defi Please check last updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants